Skip to content

Fix Android crash: view created after ViewController destroyed (MM-68032)#9693

Open
nickmisasi wants to merge 1 commit intomainfrom
cursor/fix-rnn-statusbar-destroy-5939
Open

Fix Android crash: view created after ViewController destroyed (MM-68032)#9693
nickmisasi wants to merge 1 commit intomainfrom
cursor/fix-rnn-statusbar-destroy-5939

Conversation

@nickmisasi
Copy link
Copy Markdown
Contributor

Summary

Fixes a high-volume Android production crash (RuntimeException: Tried to create view after it has already been destroyed) seen when destroying MainActivity, with the stack going through react-native-navigation status bar handling.

Root cause: In StatusBarPresenter.setStatusBarVisible, Kotlin viewController.view resolves to the Java getter getView(), not the backing field. During teardown that can call getView() on a controller whose view was already torn down and isDestroyed is true, which throws the exact message from Sentry.

Fix: Add ViewController.getExistingViewOnly() (returns the field without creating a view) and use it from StatusBarPresenter so status bar visibility uses either the existing view or window.decorView, never getView() implicitly.

Change is applied via the existing patches/react-native-navigation+7.45.0.patch.

Ticket Link

Fixes https://mattermost.atlassian.net/browse/MM-68032

Sentry: https://mattermost-mr.sentry.io/issues/6640793436/

Checklist

  • Added or updated unit tests (required for all new features)
  • Has UI changes
  • Includes text changes and localization file updates
  • Have tested against the 5 core themes to ensure consistency between them.
  • Have run E2E tests by adding label E2E/Run (or E2E/Run-iOS / E2E/Run-Android for platform-specific runs).

Device Information

This PR was tested on: not run (native Android patch; npm run tsc passed)

Screenshots

N/A

Release Note

Fixed an Android crash when closing the app related to status bar handling during activity destroy.
Open in Web Open in Cursor 

Kotlin property viewController.view maps to Java getView(), which creates
the view and throws if the controller was already destroyed during
MainActivity teardown (Sentry MATTERMOST-MOBILE-ANDROID-AX68 / MM-68032).

Add ViewController.getExistingViewOnly() and use it from StatusBarPresenter.

Refs MM-68032

Co-authored-by: Nick Misasi <nick13misasi@gmail.com>
@github-actions
Copy link
Copy Markdown

Coverage Comparison Report

Generated on April 15, 2026 at 18:27:39 UTC

+-----------------+------------+------------+-----------+
| Metric          | Main       | This PR    | Diff      |
+-----------------+------------+------------+-----------+
| Lines           |     85.17% |     85.17% |     0.00% |
| Statements      |     85.03% |     85.03% |     0.00% |
| Branches        |     72.22% |     72.22% |     0.00% |
| Functions       |     84.03% |     84.03% |     0.00% |
+-----------------+------------+------------+-----------+
| Total           |     81.61% |     81.61% |     0.00% |
+-----------------+------------+------------+-----------+

@nickmisasi nickmisasi marked this pull request as ready for review April 15, 2026 19:30
@nickmisasi nickmisasi requested a review from enahum April 15, 2026 19:30
@nickmisasi
Copy link
Copy Markdown
Contributor Author

@enahum first go at one of these - this is fully AI-driven. If it's on the right track I'm happy to pick up a few more of these off the list and run them through, if not, I will close and not waste your time

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 15, 2026

📝 Walkthrough

Walkthrough

This patch modifies react-native-navigation to extend NavigationActivity from ReactActivity instead of AppCompatActivity, refactor NavigationModule to use Optional pattern for navigator access, add a new getExistingViewOnly() API to ViewController, improve null-safety for gesture handling, add soft-keyboard padding adjustments, and update iOS frame assignments and title layout handling.

Changes

Cohort / File(s) Summary
NavigationActivity Base Class & Back-Press Handling
node_modules/react-native-navigation/lib/android/.../NavigationActivity.java
Changes superclass from AppCompatActivity to ReactActivity, disables back-press callback override logic, adjusts onActivityResult visibility to public, and removes @TargetApi annotation.
ViewController & View Access
node_modules/react-native-navigation/lib/android/.../ViewController.java
Adds new public API getExistingViewOnly() that returns the current view field without creating/recreating it.
ChildController Window & Lifecycle Handling
node_modules/react-native-navigation/lib/android/.../ChildControllersRegistry.java, node_modules/react-native-navigation/lib/android/.../ChildController.java
Adds isDestroyed() check before calling onViewBroughtToFront(), and implements soft-keyboard (IME) padding adjustments for non-root views when SOFT_INPUT_ADJUST_NOTHING is set.
NavigationModule & React Bridge
node_modules/react-native-navigation/lib/android/.../NavigationModule.java
Refactors navigator access to use Optional<Navigator> pattern with guarded calls, adds ClassCastException handling for activity-resume scenarios, changes activity() return type to Activity, and renames lifecycle method from onCatalystInstanceDestroy() to invalidate().
ReactGateway Window Focus
node_modules/react-native-navigation/lib/android/.../ReactGateway.java
Adds new public method onWindowFocusChanged(boolean) to forward window focus changes to ReactInstanceManager.
RN71 Modal Touch Handling
node_modules/react-native-navigation/lib/android/.../ModalContentLayout.java
Updates gesture and touch-related method overrides to accept nullable MotionEvent/return nullable EventDispatcher, with conditional invocation of mJSTouchDispatcher only when non-null.
iOS ViewController Frame & Layout
node_modules/react-native-navigation/lib/ios/.../RNNComponentViewController.m, node_modules/react-native-navigation/lib/ios/.../RNNTitleViewHelper.m
Updates reactView frame assignment to use UIScreen.mainScreen.bounds before setting from self.view.frame, and adjusts title/subtitle layout by computing max title width from navigation bar bounds with single-line truncating labels.
iOS Hit-Test Exclusion
node_modules/react-native-navigation/lib/ios/.../RNNOverlayWindow.m
Expands hit-test exclusion to include RNCSafeAreaView and RNCSafeAreaProvider via dynamic class lookup.
TypeScript Type Imports
node_modules/react-native-navigation/lib/typescript/Options.ts
Changes react-native imports to use import type syntax for type-only imports (no runtime behaviour change).

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested labels

kind/bug

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title directly and concisely describes the main fix: preventing a view creation crash on Android during ViewController destruction, with the ticket reference.
Description check ✅ Passed The description is fully related to the changeset, explaining the root cause, the fix, and providing context about the crash being addressed.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch cursor/fix-rnn-statusbar-destroy-5939

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
patches/react-native-navigation+7.45.0.patch (2)

263-273: ⚠️ Potential issue | 🟠 Major

Potential ClassCastException in invalidate() method

The activity() method now returns Activity instead of NavigationActivity, but invalidate() performs a direct cast without a guard. If the current activity is not a NavigationActivity, this will throw a ClassCastException. This is inconsistent with the defensive Optional pattern applied elsewhere in this file.

🐛 Proposed fix to add instanceof check
 `@Override`
 public void invalidate() {
-    final NavigationActivity navigationActivity = (NavigationActivity)activity();
-    if (navigationActivity != null) {
+    final Activity current = activity();
+    if (current instanceof NavigationActivity) {
+        final NavigationActivity navigationActivity = (NavigationActivity) current;
         navigationActivity.onCatalystInstanceDestroy();
     }
     super.invalidate();
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@patches/react-native-navigation`+7.45.0.patch around lines 263 - 273,
invalidate() currently casts activity() to NavigationActivity directly which can
throw ClassCastException; change the method to retrieve the Activity (via
activity()), check "instanceof NavigationActivity" before casting, and only call
navigationActivity.onCatalystInstanceDestroy() if the check passes; keep the
call to super.invalidate() unmodified. Reference symbols: invalidate(),
activity(), NavigationActivity, onCatalystInstanceDestroy(), super.invalidate().

353-372: ⚠️ Potential issue | 🟡 Minor

Inconsistent null-safety: !! assertion on nullable getEventDispatcher()

The getEventDispatcher() return type was changed to EventDispatcher? (nullable), but lines 357, 364, and 371 use !! (non-null assertion). If getEventDispatcher() returns null, this will throw a NullPointerException, negating the null-safety improvements.

🛡️ Proposed fix to handle nullable dispatcher consistently
     override fun onChildStartedNativeGesture(child: View, androidEvent: MotionEvent?) {
-        androidEvent?.let {
-            mJSTouchDispatcher.onChildStartedNativeGesture(it, this.getEventDispatcher()!!)
+        val dispatcher = getEventDispatcher() ?: return
+        androidEvent?.let {
+            mJSTouchDispatcher.onChildStartedNativeGesture(it, dispatcher)
         }
     }
     override fun onChildStartedNativeGesture(androidEvent: MotionEvent?) {
-        androidEvent?.let {
-            mJSTouchDispatcher.onChildStartedNativeGesture(it, this.getEventDispatcher()!!)
+        val dispatcher = getEventDispatcher() ?: return
+        androidEvent?.let {
+            mJSTouchDispatcher.onChildStartedNativeGesture(it, dispatcher)
         }
     }
     override fun onChildEndedNativeGesture(child: View, androidEvent: MotionEvent?) {
-        androidEvent?.let {
-            mJSTouchDispatcher.onChildEndedNativeGesture(it, this.getEventDispatcher()!!)
+        val dispatcher = getEventDispatcher() ?: return
+        androidEvent?.let {
+            mJSTouchDispatcher.onChildEndedNativeGesture(it, dispatcher)
         }
     }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@patches/react-native-navigation`+7.45.0.patch around lines 353 - 372, The
code uses non-null assertions on a now-nullable getEventDispatcher() inside
onChildStartedNativeGesture and onChildEndedNativeGesture; instead of using !!,
check the dispatcher for null and bail out or safely call it (e.g., val
dispatcher = getEventDispatcher() ?: return or getEventDispatcher()?.let { ...
}) before calling mJSTouchDispatcher.onChildStartedNativeGesture /
onChildEndedNativeGesture so you never force-unwrap a nullable EventDispatcher;
update both overloads of onChildStartedNativeGesture and
onChildEndedNativeGesture to use a safe-check on getEventDispatcher().
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@patches/react-native-navigation`+7.45.0.patch:
- Around line 263-273: invalidate() currently casts activity() to
NavigationActivity directly which can throw ClassCastException; change the
method to retrieve the Activity (via activity()), check "instanceof
NavigationActivity" before casting, and only call
navigationActivity.onCatalystInstanceDestroy() if the check passes; keep the
call to super.invalidate() unmodified. Reference symbols: invalidate(),
activity(), NavigationActivity, onCatalystInstanceDestroy(), super.invalidate().
- Around line 353-372: The code uses non-null assertions on a now-nullable
getEventDispatcher() inside onChildStartedNativeGesture and
onChildEndedNativeGesture; instead of using !!, check the dispatcher for null
and bail out or safely call it (e.g., val dispatcher = getEventDispatcher() ?:
return or getEventDispatcher()?.let { ... }) before calling
mJSTouchDispatcher.onChildStartedNativeGesture / onChildEndedNativeGesture so
you never force-unwrap a nullable EventDispatcher; update both overloads of
onChildStartedNativeGesture and onChildEndedNativeGesture to use a safe-check on
getEventDispatcher().

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 5d92241d-6386-4f9b-a26a-26e57762d7ec

📥 Commits

Reviewing files that changed from the base of the PR and between 4301a56 and 5d4e7bf.

📒 Files selected for processing (1)
  • patches/react-native-navigation+7.45.0.patch

@enahum
Copy link
Copy Markdown
Contributor

enahum commented Apr 15, 2026

Do we really want this? I think the effort should be placed to merge #9402

@nickmisasi
Copy link
Copy Markdown
Contributor Author

nickmisasi commented Apr 15, 2026

@enahum I'm all for refactors, in the last 60 days this crash has happened nearly 2000 times :P

If that PR is going to be merged soon then I'm happy to close the PR, but if it's going to be another month or two, I think we should fix the crash as it is

@enahum
Copy link
Copy Markdown
Contributor

enahum commented Apr 15, 2026

Lwt's leave this open for a week, let's see if the PR makes it by then (I hope it will)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants